Skip to content

Simplify BooleanConverter #366

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
basil wants to merge 1 commit into from
Closed

Simplify BooleanConverter #366

basil wants to merge 1 commit into from

Conversation

@basil
Copy link
Contributor

@basil basil commented Aug 11, 2025

Extracted from #364. Does not contain any changes to switch statements, which is the portion to which the veto applied.

  • Read the contribution guidelines for this project.
  • Read the ASF Generative Tooling Guidance if you use Artificial Intelligence (AI).
  • I used AI to create any part of, or all of, this pull request.
  • Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best-practice.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.

@basil basil mentioned this pull request Aug 11, 2025
Closed
7 tasks
garydgregory
garydgregory reviewed Aug 12, 2025
View reviewed changes
Copy link
Member

@garydgregory garydgregory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1: this PR is change for the sake of change IMO; and it's better to do the string conversion once in the constructor than using the more complex comparisons on every single call.

@basil
Copy link
Contributor Author

basil commented Aug 12, 2025

-1

@garydgregory If you have vetoed this PR, and there is no clear action item that would cause you to withdraw the veto, then should the PR be closed?

@basil basil closed this by deleting the head repository Dec 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@garydgregory garydgregory garydgregory left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@basil @garydgregory